Speedup snapshot stale indices delete #64513

piyushdaftary · 2020-11-03T04:53:15Z

Have you signed the contributor license agreement? Yes
Have you followed the contributor guidelines? Yes
If submitting code, have you built your formula locally prior to submission with gradle check? Yes
If submitting code, is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed. Yes
If submitting code, have you checked that your submission is for an OS and architecture that we support?Yes
If you are submitting this code for a class then read our policy for that. Yes

Current implementation cleanupStaleIndices() of snapshot deletion is very slow . Snapshot deletion code deletes each stale indices from repository one after another sequentially .

With this code changes instead of making snapshot delete of stale indices a single threaded operation we make it multithreaded operation and delete multiple stale indices in parallel using SNAPSHOT thread pool's workers.

I have added more tests to make sure outage scenario and failover scenario is handled.

Fixes elastic#61513

elasticmachine · 2020-11-03T05:14:28Z

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

original-brownbear

Thanks @piyushdaftary , one ask :)

server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java

…ing manner

original-brownbear · 2020-12-01T10:21:24Z

Thanks @piyushdaftary and sorry for letting this fall of my radar for a bit. I'll do my best to review this one today.

original-brownbear

Very nice work @piyushdaftary ! Sorry again it took me so long to review this.

Just a few open details here, but the general solution is exactly what I had in mind, thanks!

server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java

server/src/internalClusterTest/java/org/elasticsearch/snapshots/RepositoriesIT.java

original-brownbear · 2020-12-12T21:27:27Z

Jenkins test this

original-brownbear · 2020-12-12T21:40:59Z

Jenkins run elasticsearch-ci/bwc (dep download failure only)

original-brownbear · 2020-12-12T21:48:00Z

@piyushdaftary thanks this looks really nice now, could you please fix the checkstyle issues that are currently failing CI runs:

22:34:09 [ant:checkstyle] [ERROR] /dev/shm/elastic+elasticsearch+pull-request-1/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RepositoriesIT.java:24:8: Unused import - org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse. [UnusedImports]
22:34:09 [ant:checkstyle] [ERROR] /dev/shm/elastic+elasticsearch+pull-request-1/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RepositoriesIT.java:48:15: Unused import - org.hamcrest.Matchers.greaterThan. [UnusedImports]
22:34:09

(just some unused imports in the test class I think). You can make sure they all got fixed locally by running ./gradlew precommit btw.

Thanks again!

piyushdaftary · 2020-12-13T03:37:38Z

@piyushdaftary thanks this looks really nice now, could you please fix the checkstyle issues that are currently failing CI runs:
22:34:09 [ant:checkstyle] [ERROR] /dev/shm/elastic+elasticsearch+pull-request-1/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RepositoriesIT.java:24:8: Unused import - org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse. [UnusedImports]
22:34:09 [ant:checkstyle] [ERROR] /dev/shm/elastic+elasticsearch+pull-request-1/server/src/internalClusterTest/java/org/elasticsearch/snapshots/RepositoriesIT.java:48:15: Unused import - org.hamcrest.Matchers.greaterThan. [UnusedImports]
22:34:09 
(just some unused imports in the test class I think). You can make sure they all got fixed locally by running ./gradlew precommit btw.

Thanks again!

Thanks @original-brownbear.
Removed the unused imports.

original-brownbear · 2020-12-13T14:05:04Z

Jenkins test this

original-brownbear · 2020-12-13T21:03:37Z

@piyushdaftary thanks, looks good :)

There is one test failure here: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+pull-request-1/14390/testReport/junit/org.elasticsearch.repositories.fs/FsBlobStoreRepositoryIT/testIndicesDeletedFromRepository/ that looks related to the changes unfortunately. Could you look into this one please? The failure has instructions on how to reproduce it locally, check the reproduce with message in the CI logs. Let me know if you need any help with this, then I'll see if I can track it down.

piyushdaftary · 2020-12-14T02:13:23Z

Hi @original-brownbear
I tried to reproduce the test failure org.elasticsearch.repositories.fs.FsBlobStoreRepositoryIT.testIndicesDeletedFromRepository locally, but unfortunately I am unable to reproduce it on my machine.

Below is cmd I am trying to run the test with, it succeeds every time for me :

38f9d37053af:elasticsearch pdaftary$ ./gradlew ':server:internalClusterTest' --tests "org.elasticsearch.repositories.fs.FsBlobStoreRepositoryIT.testIndicesDeletedFromRepository" -Dtests.seed=A6BCE803F5F88E4C -Dtests.security.manager=true -Dtests.locale=es-SV -Dtests.timezone=Australia/Canberra -Druntime.java=11
=======================================
Elasticsearch Build Hamster says Hello!
  Gradle Version        : 6.6.1
  OS Info               : Mac OS X 10.14.6 (x86_64)
  Runtime JDK Version   : 11 (Oracle JDK)
  Runtime java.home     : /Library/Java/JavaVirtualMachines/jdk-11.0.5.jdk/Contents/Home
  Gradle JDK Version    : 14 (OpenJDK)
  Gradle java.home      : /Library/Java/JavaVirtualMachines/jdk-14.0.1.jdk/Contents/Home
  Random Testing Seed   : A6BCE803F5F88E4C
  In FIPS 140 mode      : false
=======================================

BUILD SUCCESSFUL in 1m 5s

piyushdaftary · 2020-12-16T06:44:11Z

@original-brownbear : Can you please let me know how else I can reproduce the issue on my local machine. Till date the test org.elasticsearch.repositories.fs.FsBlobStoreRepositoryIT.testIndicesDeletedFromRepository is passing in my machine when ran against my branch code.

Do I need to merge in latest elastic/elasticsearch - master branch code to reproduce it ?

original-brownbear · 2020-12-16T07:22:07Z

@piyushdaftary

sorry for the delay here, had a few high pressure things incoming at the start of this week, will do my best to look into this today.

Do I need to merge in latest elastic/elasticsearch - master branch code to reproduce it ?

No, CI runs your exact code without any merging beforehand so it should be reproducible in theory. In practice it's often a little tricky :) I'll try to think through the change today, could be we've just change some timing in such a way that it surfaced an existing issue. I'm on it later.

server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java

in deleting snapshot

original-brownbear · 2020-12-19T11:56:24Z

Jenkins test this

original-brownbear · 2020-12-19T13:40:21Z

@piyushdaftary this looks good, tests seem to be passing now. Could you merge in latest master though? I think the fact that we're out of data a little there is causing some tests to fail again.

Sync from elastic master

piyushdaftary · 2020-12-19T15:22:28Z

@original-brownbear : Thanks for reviewing code.
I have merged in latest master code to my branch.

original-brownbear · 2020-12-19T15:34:36Z

@elasticmachine test this

original-brownbear

LGTM thanks @piyushdaftary! Looks very nice now, I'll merge this in the coming days.

piyushdaftary · 2021-01-11T21:45:09Z

@original-brownbear: Thanks for approving the code changes. By when will it be merged to master ?

DaveCTurner · 2021-09-14T07:52:44Z

@original-brownbear looks like this one has slipped off the radar.

pugnascotia · 2022-03-18T16:49:54Z

@original-brownbear are you still looking at this?

DaveCTurner · 2023-10-05T05:05:51Z

Sorry this has taken so long @piyushdaftary, this PR is now rather stale. I've opened one afresh at #100316 and marked you as a co-author. Closing this.

Speedup snapshot stale indices delete

2dfbb9c

Fixes elastic#61513

tvernum added the :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs label Nov 3, 2020

elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Nov 3, 2020

original-brownbear self-requested a review November 3, 2020 06:29

original-brownbear requested changes Nov 4, 2020

View reviewed changes

server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java Outdated Show resolved Hide resolved

piyushdaftary added 2 commits November 29, 2020 05:45

Making delete stale indices to be pulled form a queue in a work-steal…

9690339

…ing manner

removing redundant newLine

c11faa7

piyushdaftary requested a review from original-brownbear December 1, 2020 10:20

original-brownbear requested changes Dec 2, 2020

View reviewed changes

resolving comments

bb25297

piyushdaftary requested a review from original-brownbear December 12, 2020 12:37

removing unused imports

a4200c7

original-brownbear reviewed Dec 18, 2020

View reviewed changes

server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java Outdated Show resolved Hide resolved

Adding check to make sure found incies are not part of surviving indices

d7e8cc5

in deleting snapshot

Merge pull request #1 from elastic/master

f422c71

Sync from elastic master

original-brownbear approved these changes Dec 19, 2020

View reviewed changes

occidere mentioned this pull request Jan 25, 2021

진정한 오픈소스 Elasticsearch 를 위한 발돋움 (Stepping up for a truly open source Elasticsearch) occidere/TIL#161

Open

This was referenced Apr 25, 2021

Make snapshot deletion faster opensearch-project/OpenSearch#147

Closed

Speedup snapshot stale indices delete opensearch-project/OpenSearch#613

Merged

AmiStrn mentioned this pull request Feb 24, 2022

Parallelize stale blobs deletion during snapshot delete opensearch-project/OpenSearch#2159

Closed

2 tasks

pugnascotia assigned original-brownbear Mar 18, 2022

elasticsearchmachine changed the base branch from master to main July 22, 2022 23:12

DaveCTurner closed this Oct 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Speedup snapshot stale indices delete #64513

Speedup snapshot stale indices delete #64513

piyushdaftary commented Nov 3, 2020

elasticmachine commented Nov 3, 2020

original-brownbear left a comment •

edited

Loading

original-brownbear commented Dec 1, 2020 •

edited

Loading

original-brownbear left a comment

original-brownbear commented Dec 12, 2020

original-brownbear commented Dec 12, 2020

original-brownbear commented Dec 12, 2020

piyushdaftary commented Dec 13, 2020

original-brownbear commented Dec 13, 2020

original-brownbear commented Dec 13, 2020

piyushdaftary commented Dec 14, 2020 •

edited

Loading

piyushdaftary commented Dec 16, 2020 •

edited

Loading

original-brownbear commented Dec 16, 2020

original-brownbear commented Dec 19, 2020

original-brownbear commented Dec 19, 2020

piyushdaftary commented Dec 19, 2020

original-brownbear commented Dec 19, 2020

original-brownbear left a comment

piyushdaftary commented Jan 11, 2021 •

edited

Loading

DaveCTurner commented Sep 14, 2021

pugnascotia commented Mar 18, 2022

DaveCTurner commented Oct 5, 2023

Speedup snapshot stale indices delete #64513

Speedup snapshot stale indices delete #64513

Conversation

piyushdaftary commented Nov 3, 2020

elasticmachine commented Nov 3, 2020

original-brownbear left a comment • edited Loading

Choose a reason for hiding this comment

original-brownbear commented Dec 1, 2020 • edited Loading

original-brownbear left a comment

Choose a reason for hiding this comment

original-brownbear commented Dec 12, 2020

original-brownbear commented Dec 12, 2020

original-brownbear commented Dec 12, 2020

piyushdaftary commented Dec 13, 2020

original-brownbear commented Dec 13, 2020

original-brownbear commented Dec 13, 2020

piyushdaftary commented Dec 14, 2020 • edited Loading

piyushdaftary commented Dec 16, 2020 • edited Loading

original-brownbear commented Dec 16, 2020

original-brownbear commented Dec 19, 2020

original-brownbear commented Dec 19, 2020

piyushdaftary commented Dec 19, 2020

original-brownbear commented Dec 19, 2020

original-brownbear left a comment

Choose a reason for hiding this comment

piyushdaftary commented Jan 11, 2021 • edited Loading

DaveCTurner commented Sep 14, 2021

pugnascotia commented Mar 18, 2022

DaveCTurner commented Oct 5, 2023

original-brownbear left a comment •

edited

Loading

original-brownbear commented Dec 1, 2020 •

edited

Loading

piyushdaftary commented Dec 14, 2020 •

edited

Loading

piyushdaftary commented Dec 16, 2020 •

edited

Loading

piyushdaftary commented Jan 11, 2021 •

edited

Loading